Skip to content

docs(accuracysnes): adjudicate ares' divergences; two corrections to my own claims - #305

Merged
doublegate merged 1 commit into
mainfrom
docs/ares-adjudication
Aug 1, 2026
Merged

doublegate merged 1 commit into
mainfrom
docs/ares-adjudication

Conversation

@doublegate

@doublegate doublegate commented Aug 1, 2026 •

Copy link
Copy Markdown
Owner

Follow-up to #304, which established the ares host and reported five divergences without adjudicating them.

Three of the five are rows snes9x already fails

The tally alone does not show this, and it changes what each row means:

row ares code who else fails it so the split is
C7.05 1 snes9x (code 2 — a different failure) 2-vs-2, and the two dissenters disagree with each other
C7.10 1 snes9x 2-vs-2 — RustySNES + Mesen2 against snes9x + ares
F1.10 2 snes9x 2-vs-2 (see below)
E8.02 3 nobody ares alone
E3.06 2 nobody ares alone

So ares is corroborating snes9x on three rows rather than standing alone. Still not wired into crossval.sh: an ARES_KNOWN_FAILURES constant encoding unadjudicated disagreements would be worse than no third reference.

Correction 1 — to my own README and to PR #304

I wrote that F1.10 is a PAD2_CONTRACT row and that this host'''s port detection should be suspected first. That is wrong. f1_require_contract reads $4016 only — port 1 — and F1.10 code 2 means "$4212 read busy at the very start of the vblank line", which does not involve controller state at all. I inherited the claim from crossval.sh'''s Mesen2 grouping instead of checking it.

Correction 2 — to crossval.sh

Its MESEN2_KNOWN_FAILURES comment names idx279 F1.03 and idx286 F1.10. In the current catalogue those indices are F1.01 and F1.08 — an index moves whenever a test is added ahead of it. Now keyed on row names, with the drift stated: an index in a comment is a fact with a shelf life.

That same comment attributes Mesen2'''s F1.10 failure to the port-2 limitation, while the snes9x block a few lines above says Mesen2 passes F1.10. Both cannot be true. Marked as doubted rather than quietly rewritten — resolving it needs Mesen2'''s failing set read at DONE and mapped through SOURCE_CATALOG.tsv, a measurement nobody has taken since the catalogue grew.

And F1.10 now deserves a hard look

fullsnes puts the automatic read'''s start ~dot 32.5–95.5 into the first vblank line rather than at the vblank edge. snes9x fails that row; ares fails it; and if the Mesen2 attribution is right, Mesen2 fails it too — leaving RustySNES passing alone, on a row it passes only because of a deliberate fix. This project'''s heuristic says RustySNES failing alone means a real bug. It should say the same about RustySNES passing alone.

Verification

REF_PROJ=$PWD/ref-proj bash scripts/accuracysnes/crossval.sh — unchanged: snes9x: OK (14 known), Mesen2: OK (2 known), 54 scenes match on both, 2 reference(s) agree with the cart. bash -n clean. Docs and comments only.

🤖 Generated with Claude Code

Summary

This change updates AccuracySNES dossier assertions from ares comparison results.

  • The dossier now claims that three of five ares failures also fail in snes9x. E8.02 and E3.06 remain ares-only. This claim is false if fresh comparison results change either the overlap or the ares-only rows.
  • The dossier now claims that F1.10 reads $4016, while code 2 concerns $4212 busy status at vblank start. The previous PAD2_CONTRACT attribution is false.
  • Mesen2 references now use stable row names. The F1.10 attribution remains unresolved because recorded behavior conflicts. The claim is false if fresh measurement confirms one attribution.
  • The dossier flags F1.10 for investigation because RustySNES may pass the row alone despite the deliberate fix. This claim is false if isolated testing shows RustySNES fails or the fix is not causal.
  • Coverage remains 54 matching scenes. The coverage denominator did not move.
  • Verification remains unchanged: both reference hosts pass, and shell syntax checks pass.

…my own claims

Three of ares' five failures are rows snes9x ALREADY fails, which the tally
alone does not show and which changes what each one means: C7.10 and F1.10
become 2-vs-2 (RustySNES + Mesen2 against snes9x + ares), C7.05 is 2-vs-2 with
the two dissenters failing on different codes, and only E8.02 and E3.06 are
ares-only. ares is corroborating snes9x more than it is standing alone. Still
not wired into crossval.sh -- an ARES_KNOWN_FAILURES constant encoding
unadjudicated disagreements would be worse than no third reference.

CORRECTION 1, to the ares README and PR #304: F1.10 is NOT a PAD2_CONTRACT
row and this host's port detection is not the suspect. f1_require_contract
reads $4016 only -- port 1 -- and F1.10 code 2 means "$4212 read busy at the
very start of the vblank line", which does not involve controller state. The
claim was inherited from crossval.sh's Mesen2 grouping rather than checked.

CORRECTION 2, to crossval.sh itself: its Mesen2 known-failure comment names
idx279 F1.03 and idx286 F1.10, and in the current catalogue those indices are
F1.01 and F1.08 -- an index moves whenever a test is added ahead of it. Keyed
on rows now, with the drift noted. That same comment attributes Mesen2's F1.10
failure to the port-2 limitation while the snes9x block a few lines above says
Mesen2 PASSES F1.10; both cannot be true, and it is marked as doubted rather
than quietly rewritten, because resolving it needs Mesen2's failing set read
at DONE and mapped through SOURCE_CATALOG.tsv.

F1.10 now deserves a hard look on its own: fullsnes puts the automatic read's
start ~dot 32.5-95.5 into the first vblank line, snes9x fails the row, ares
fails it, and if the Mesen2 attribution is right then RustySNES passes ALONE
on a row it passes only because of a deliberate fix. The heuristic that says
RustySNES failing alone means a real bug should say the same about RustySNES
passing alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 1, 2026 11:28
@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The PR updates AccuracySNES documentation with detailed ares and snes9x disagreement results, revises the unresolved F1.10 explanation, and replaces unstable Mesen2 catalogue indices with stable row names.

Changes

AccuracySNES documentation

Layer / File(s) Summary
ares disagreement findings
scripts/accuracysnes/ares_host/README.md, CHANGELOG.md
The ares comparison identifies shared snes9x failures, ares-only failures, vote splits, and unresolved $4212 vblank timing behavior for F1.10.
Cross-validation divergence references
scripts/accuracysnes/crossval.sh, CHANGELOG.md
Mesen2 references now use F1.03 and F1.10 row names. The F1.10 attribution is marked unverified.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 10
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses the valid docs type and scope, uses imperative mood, has no trailing period, and accurately describes the documentation corrections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Changelog Entry ✅ Passed The full PR diff from origin/main includes substantive CHANGELOG.md hunks under Unreleased and Changed.
Docs-As-Spec ✅ Passed Compared with the base commit, the PR changes only CHANGELOG.md and AccuracySNES documentation/comments; it changes no crates/** code or observable behavior.
Accuracysnes Bookkeeping ✅ Passed The base-to-HEAD diff changes only three documentation/comment files; no test or scene under tests/roms/AccuracySNES/gen/src/ changed, so bookkeeping checks do not apply.
No Panic On Untrusted Input ✅ Passed The patch changes only CHANGELOG.md, a README, and comments in crossval.sh; no new .unwrap(), .expect(), or panic!() calls were added.
Safety Comment On New Unsafe ✅ Passed The patch changes only CHANGELOG.md and AccuracySNES documentation/comments; no new unsafe tokens appear, and Rust unsafe count is unchanged at 70.

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Antigravity review (Gemini via Ultra)

This PR updates documentation and shell script comments across CHANGELOG.md, scripts/accuracysnes/ares_host/README.md, and scripts/accuracysnes/crossval.sh to refine the classification of ares test row disagreements and retract prior assumptions regarding test F1.10.

Blocking issues

None found.

Suggestions

  • PR Title / Subject Line: The PR subject docs(accuracysnes): adjudicate ares' divergences; two corrections to my own claims is 78 characters (exceeding the 72-character project limit) and inaccurate: the diff explicitly states that these divergences remain unadjudicated, directly contradicting the title word "adjudicate".
  • scripts/accuracysnes/crossval.sh:174-192: Preserving the inaccurate F1.10 attribution in the comment block while adding a second comment block explaining why it is doubted creates internal contradictions in the script; map and correct the failure attribution directly.

Nitpicks

  • scripts/accuracysnes/ares_host/README.md:56-61: Commentary on F1.10 ("Worth a hard look before treating it as settled...") reads like a personal log entry rather than documentation; file an issue to track the audit.

Automated first-pass review by agy on a self-hosted runner -- not a human review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refines the AccuracySNES cross-validation documentation around ares/snes9x/Mesen2 divergences, correcting earlier misattribution of F1.10 and making the Mesen2 “known failures” commentary resilient to catalogue index drift.

Changes:

  • Update crossval.sh’s Mesen2 known-failure notes to be keyed by row name (not moving catalogue indices) and explicitly flag the F1.10 attribution as doubtful.
  • Revise the ares host README to reflect that 3/5 ares divergences are corroborated by snes9x (changing the interpretation from “ares-alone” to several 2-vs-2 splits) and correct the prior F1.10/PAD2_CONTRACT claim.
  • Mirror the above clarifications in the changelog for discoverability.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
scripts/accuracysnes/crossval.sh Comment-only clarification: use stable row identifiers instead of catalogue indices; mark F1.10 attribution as doubtful.
scripts/accuracysnes/ares_host/README.md Documentation correction and reframing of the five ares divergences, including the F1.10 attribution fix.
CHANGELOG.md Changelog entry updated to reflect the same adjudication/attribution corrections and index-drift warning.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 27-38: Revise the CHANGELOG discussion of F1.10 so the Mesen2
failure remains explicitly conditional rather than established. Update the
statement that identifies E8.02 and E3.06 as the only ares-only rows to
acknowledge that F1.10 is also not ares-only if Mesen2’s recorded failure is
confirmed, while preserving the later attribution doubt.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b74a340e-6e84-4d57-8822-0495d89b9549

📥 Commits

Reviewing files that changed from the base of the PR and between 6d43f87 and a1b4361.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • scripts/accuracysnes/ares_host/README.md
  • scripts/accuracysnes/crossval.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
  • GitHub Check: test-light
  • GitHub Check: lint
  • GitHub Check: accuracysnes
  • GitHub Check: copilot-pull-request-reviewer
  • GitHub Check: review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{rs,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Chip-behavior changes must update both the chip implementation and the corresponding docs/<subsystem>.md documentation.

A chip change must update both the chip implementation and its corresponding docs/<chip>.md documentation in the same change.

Files:

  • scripts/accuracysnes/ares_host/README.md
  • CHANGELOG.md
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Do not commit or vendor the generated snesdev_wiki/ mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits: <type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevant to-dos/ sprint file.

**/*: Preserve the one-directional crate graph: chip crates must not depend on one another; rustysnes-core ties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keep docs/STATUS.md as the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNES v2.0 or engine-lineage anchors as project releases.

Files:

  • scripts/accuracysnes/ares_host/README.md
  • scripts/accuracysnes/crossval.sh
  • CHANGELOG.md
scripts/accuracysnes/**

⚙️ CodeRabbit configuration file

scripts/accuracysnes/**: The cross-validation harness: the same AccuracySNES image is run on snes9x (through a
libretro host in C) and on Mesen2 (through its test runner and a Lua script), and their
verdicts are compared with the cart's. Its integrity is the whole argument for the
battery, so flag anything that could make a reference appear to agree — a verdict parsed
loosely, a missing-file path that degrades to success, a scene comparison that skips
rather than fails when the golden is absent. A known reference divergence belongs in
SNES9X_KNOWN_FAILURES with a source citation, never in a widened match.

Files:

  • scripts/accuracysnes/ares_host/README.md
  • scripts/accuracysnes/crossval.sh
**/*.md

⚙️ CodeRabbit configuration file

**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).

Files:

  • scripts/accuracysnes/ares_host/README.md
  • CHANGELOG.md
CHANGELOG.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

User-visible changes must be recorded under the [Unreleased] section.

For the full pull request diff against its base branch, modify CHANGELOG.md when user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.

Files:

  • CHANGELOG.md
🔇 Additional comments (3)
scripts/accuracysnes/ares_host/README.md (1)

34-61: LGTM!

CHANGELOG.md (1)

814-824: LGTM!

scripts/accuracysnes/crossval.sh (1)

174-194: LGTM!

Comment thread CHANGELOG.md
Comment on lines +27 to +38
**Five rows where ares disagrees with the cart** — and **three of them are rows snes9x already
fails**, which the tally alone does not show. `C7.10` and `F1.10` become **2-vs-2** (RustySNES +
Mesen2 against snes9x + ares); `C7.05` is 2-vs-2 with the two dissenters failing on *different*
codes; only `E8.02` and `E3.06` are ares-only. So ares is corroborating snes9x more than it is
standing alone. Not wired into `crossval.sh` — an `ARES_KNOWN_FAILURES` constant encoding
unadjudicated disagreements would be worse than no third reference.

**`F1.10` deserves a hard look.** fullsnes puts the automatic read's start ~dot 32.5-95.5 into the
first vblank line rather than at the vblank edge. snes9x fails that row, ares fails it, and
`crossval.sh` records Mesen2 failing it too — which would leave **RustySNES passing alone**, on a
row it passes only because of a deliberate fix. The standing heuristic says RustySNES failing
alone means a real bug; it should say the same about RustySNES *passing* alone.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Keep the F1.10 result explicitly conditional.

This entry states that only E8.02 and E3.06 are ares-only, then treats Mesen2's F1.10 failure as established. The later entry from Line 819 through Line 824 marks that attribution as doubtful. If Mesen2 really fails F1.10, that row is not ares-only.

Proposed wording
-  codes; only `E8.02` and `E3.06` are ares-only. So ares is corroborating snes9x more than it is
+  codes; only `E8.02` and `E3.06` are ares-only in the measured set; `F1.10` remains unresolved.
+  So ares is corroborating snes9x more than it is

-  `crossval.sh` records Mesen2 failing it too — which would leave **RustySNES passing alone**, on a row
+  `crossval.sh` attributes a failure to Mesen2 — if that attribution is correct, **RustySNES would be
+  passing alone**, on a row

As per path instructions, **/*.md: “Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes rather than style nits.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
**Five rows where ares disagrees with the cart** — and **three of them are rows snes9x already
fails**, which the tally alone does not show. `C7.10` and `F1.10` become **2-vs-2** (RustySNES +
Mesen2 against snes9x + ares); `C7.05` is 2-vs-2 with the two dissenters failing on *different*
codes; only `E8.02` and `E3.06` are ares-only. So ares is corroborating snes9x more than it is
standing alone. Not wired into `crossval.sh` — an `ARES_KNOWN_FAILURES` constant encoding
unadjudicated disagreements would be worse than no third reference.
**`F1.10` deserves a hard look.** fullsnes puts the automatic read's start ~dot 32.5-95.5 into the
first vblank line rather than at the vblank edge. snes9x fails that row, ares fails it, and
`crossval.sh` records Mesen2 failing it too — which would leave **RustySNES passing alone**, on a
row it passes only because of a deliberate fix. The standing heuristic says RustySNES failing
alone means a real bug; it should say the same about RustySNES *passing* alone.
**Five rows where ares disagrees with the cart** — and **three of them are rows snes9x already
fails**, which the tally alone does not show. `C7.10` and `F1.10` become **2-vs-2** (RustySNES +
Mesen2 against snes9x + ares); `C7.05` is 2-vs-2 with the two dissenters failing on *different*
codes; only `E8.02` and `E3.06` are ares-only in the measured set; `F1.10` remains unresolved.
So ares is corroborating snes9x more than it is
standing alone. Not wired into `crossval.sh` — an `ARES_KNOWN_FAILURES` constant encoding
unadjudicated disagreements would be worse than no third reference.
**`F1.10` deserves a hard look.** fullsnes puts the automatic read's start ~dot 32.5-95.5 into the
first vblank line rather than at the vblank edge. snes9x fails that row, ares fails it, and
`crossval.sh` attributes a failure to Mesen2 — if that attribution is correct, **RustySNES would be
passing alone**, on a row it passes only because of a deliberate fix. The standing heuristic says RustySNES failing
alone means a real bug; it should say the same about RustySNES *passing* alone.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CHANGELOG.md` around lines 27 - 38, Revise the CHANGELOG discussion of F1.10
so the Mesen2 failure remains explicitly conditional rather than established.
Update the statement that identifies E8.02 and E3.06 as the only ares-only rows
to acknowledge that F1.10 is also not ares-only if Mesen2’s recorded failure is
confirmed, while preserving the later attribution doubt.

Source: Path instructions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants